Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix kubeConfigPath #6568

Merged
merged 2 commits into from
Feb 10, 2020
Merged

Fix kubeConfigPath #6568

merged 2 commits into from
Feb 10, 2020

Conversation

anencore94
Copy link
Contributor

@anencore94 anencore94 commented Feb 10, 2020 •
edited
Loading

Some codes used kubeConfigPath just as $HOME/.kube/config,
but it may be different if KUBECONFIG env was set.
Thus, modify those codes to use kubeConfigPath by
using kubeconfig.PathFromEnv function.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 10, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @anencore94. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 10, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anencore94
To complete the pull request process, please assign josedonizetti
You can assign the PR to them by writing /assign @josedonizetti in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@anencore94 anencore94 force-pushed the modifyConstantConfigPath branch 2 times, most recently from 36f7e42 to 4032f69 Compare February 10, 2020 03:53
@codecov-io
Copy link

Codecov Report

Merging #6568 into master will increase coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6568      +/-   ##
==========================================
+ Coverage    38.1%   38.12%   +0.02%     
==========================================
  Files         137      137              
  Lines        8690     8685       -5     
==========================================
  Hits         3311     3311              
+ Misses       4963     4958       -5     
  Partials      416      416
Impacted Files Coverage Δ
cmd/minikube/cmd/update-context.go 0% <0%> (ø) ⬆️
cmd/minikube/cmd/stop.go 0% <0%> (ø) ⬆️
cmd/minikube/cmd/config/profile.go 0% <0%> (ø) ⬆️

@anencore94 anencore94 mentioned this pull request Feb 10, 2020
Merged
anencore94 and others added 2 commits February 10, 2020 16:51
@sophalHong @woohhan
Fix to kubeConfigPath could be set from env
2080f7b 
Fix to use kubeConfigPath by using kubeconfig.PathFromEnv
function not from just $HOME/.kube/config

Co-authored-by: sopahlHong <[email protected]>
Co-authored-by: woohhan <[email protected]>
Signed-off-by: anencore94 <[email protected]>
@sophalHong @woohhan
Delete redundant logic to get kubeConfigPath
eaa8891 
- change to use already present util function
 to set up kubeConfigPath

Co-authored-by: sopahlHong <[email protected]>
Co-authored-by: woohhan <[email protected]>
Signed-off-by: anencore94 <[email protected]>
@anencore94 anencore94 changed the title WIP : Fix to kubeConfigPath could be set from env Fix to kubeConfigPath could be set from env Feb 10, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 10, 2020
@medyagh
Copy link
Member

medyagh commented Feb 10, 2020 •
edited
Loading

@anencore94 I was hoping someone would make this PR ! thank you for doing it

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 10, 2020
@minikube-pr-bot
Copy link

All Times minikube: [ 92.858324 94.864848 92.499470]
All Times Minikube (PR 6568): [ 94.967465 91.228723 94.233977]

Average minikube: 93.407547
Average Minikube (PR 6568): 93.476722

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 6568) |
+----------------------+-----------+--------------------+
| minikube v           |  0.205257 |           0.209616 |
| Creating kvm2        | 20.036816 |          20.237842 |
| Preparing Kubernetes | 50.349434 |          50.419197 |
| Pulling images       |           |                    |
| Launching Kubernetes | 20.979452 |          20.817415 |
| Waiting for cluster  |  0.224404 |           0.217580 |
+----------------------+-----------+--------------------+

@medyagh medyagh merged commit a283fd6 into kubernetes:master Feb 10, 2020
@medyagh
Copy link
Member

medyagh commented Feb 10, 2020

thank you @anencore94

@medyagh medyagh changed the title Fix to kubeConfigPath could be set from env Fix kubeConfigPath Feb 10, 2020
medyagh pushed a commit to medyagh/minikube that referenced this pull request Feb 10, 2020
@anencore94 @sophalHong
@woohhan @medyagh
Fix to kubeConfigPath could be set from env (kubernetes#6568)
3929611 
* Fix to kubeConfigPath could be set from env

Fix to use kubeConfigPath by using kubeconfig.PathFromEnv
function not from just $HOME/.kube/config

Co-authored-by: sopahlHong <[email protected]>
Co-authored-by: woohhan <[email protected]>
Signed-off-by: anencore94 <[email protected]>

* Delete redundant logic to get kubeConfigPath

- change to use already present util function
 to set up kubeConfigPath

Co-authored-by: sopahlHong <[email protected]>
Co-authored-by: woohhan <[email protected]>
Signed-off-by: anencore94 <[email protected]>

Co-authored-by: SOPHAL HONG <[email protected]>
Co-authored-by: Woohyung Han <[email protected]>
@anencore94 anencore94 deleted the modifyConstantConfigPath branch February 11, 2020 01:01
@fmotrifork fmotrifork mentioned this pull request Feb 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@medyagh medyagh Awaiting requested review from medyagh

@tstromberg tstromberg Awaiting requested review from tstromberg

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@anencore94 @k8s-ci-robot @minikube-bot @codecov-io @medyagh @minikube-pr-bot